Skip to content

fix: preserve the context state during evaluate — fixes #90, #93#103

Merged
uw4 merged 4 commits intodashjoin:mainfrom
matanzwix:fix/thread-safety-eval-context
Apr 29, 2026
Merged

fix: preserve the context state during evaluate — fixes #90, #93#103
uw4 merged 4 commits intodashjoin:mainfrom
matanzwix:fix/thread-safety-eval-context

Conversation

@matanzwix
Copy link
Copy Markdown
Contributor

@matanzwix matanzwix commented Mar 28, 2026

Problem

Two related thread-safety bugs make it unsafe to cache and reuse parsed Jsonata instances:

Issue #93 — Bindings leak across instances on the same thread

The constructor sets current.set(this) on a single static ThreadLocal<Jsonata>. When a second instance is constructed on the same thread, it overwrites the first. Subsequent evaluations of the first instance can silently use the second instance's per-thread evaluator state, causing environment bindings to leak across unrelated expressions.

Issue #90$eval() returns null for nested/deep contexts

_evaluate() stores evaluation state such as input and environment as mutable instance fields during recursive evaluation. When $eval() reads that state through Jsonata.current.get(), it can observe whatever the most recent recursive _evaluate() call wrote, rather than the correct calling context for that $eval() invocation. This causes cases like $eval($.funcs.func) to return null instead of the expected result.

Both bugs stem from the same root cause: per-evaluation mutable state stored on the shared Jsonata instance and accessed through a single static ThreadLocal<Jsonata>.

Solution (minimal fix)

Preserve the context during evaluate on the stack.
Thus we can keep the original code structure and minimize the # of changed lines of code

Original Solution (not merged - used minimal fix)

Extract all per-evaluation mutable state into a lightweight EvalContext value object stored in a single static ThreadLocal<EvalContext>.

This separates:

  • the parsed expression and base configuration on Jsonata
  • the live execution state for the current evaluation

As a result, Jsonata no longer stores per-evaluation mutable state, and parsed expressions can be safely cached and reused across threads after setup.

What changed

Jsonata.java

Before After
static ThreadLocal<Jsonata> current static ThreadLocal<EvalContext> evalContext
Per-evaluation mutable state stored on Jsonata (input, timestamp, current eval state) Per-evaluation mutable state stored on EvalContext
getPerThreadInstance() + clone per thread Removed
Copy constructor Jsonata(Jsonata other) Removed
Constructor sets current.set(this) Removed
_evaluate() overwrites mutable state on this _evaluate() saves/restores state on EvalContext in try/finally
evaluate(Object, Frame) does not manage a dedicated execution context evaluate(Object, Frame) creates/restores EvalContext

Functions.java

  • All Jsonata.current.get() call sites now read from Jsonata.evalContext.get()
  • $eval(), $now(), $millis(), and lambda application now use EvalContext
  • funcApply() no longer performs redundant repeated ThreadLocal.get() calls

Design

Before:
  Jsonata instance held mutable evaluation state
  static ThreadLocal<Jsonata>
  per-thread cloning

After:
  Jsonata holds parsed expression + base configuration
  static ThreadLocal<EvalContext>
  no cloning

…n#90, dashjoin#93)

Replace the broken static ThreadLocal<Jsonata> with a lightweight
EvalContext value object on a single static ThreadLocal. The Jsonata
instance is now immutable after construction and freely shareable
across threads with no cloning.
@matanzwix
Copy link
Copy Markdown
Contributor Author

@uw4 @aeberhart could you please take a look?

@uw4
Copy link
Copy Markdown
Contributor

uw4 commented Apr 20, 2026

Thanks for the excellent contribution + providing the test cases, this is an excellent result!

However we try to minimize the # of changes/changed lines of code, and also try to stay structurally as near as possible with the original Javascript.

Thus while reproducing your work, it turned out that this is the only change required in order to fix the issues and pass all test cases (and also has no material effect on any benchmarks):

diff --git a/src/main/java/com/dashjoin/jsonata/Jsonata.java b/src/main/java/com/dashjoin/jsonata/Jsonata.java
index d60bf19..fe87c9f 100644
--- a/src/main/java/com/dashjoin/jsonata/Jsonata.java
+++ b/src/main/java/com/dashjoin/jsonata/Jsonata.java
@@ -129,7 +129,15 @@ public class Jsonata {
     Object evaluate(Symbol expr, Object input, Frame environment) {
         // Thread safety:
         // Make sure each evaluate is executed on an instance per thread
-        return getPerThreadInstance()._evaluate(expr, input, environment);
+        Jsonata _this = getPerThreadInstance();
+        Object _input = _this.input;
+        Frame _environment = _this.environment;
+        try {
+          return _this._evaluate(expr, input, environment);
+        } finally {
+            _this.input = _input;
+            _this.environment = _environment;
+        }
     }
 
     Object _evaluate(Symbol expr, Object input, Frame environment) {
  • Can you please just use this minimal change in the pull request (along with the unit tests)?
    Then I would merge it so you are preserved as commiter

uw4 added 2 commits April 29, 2026 18:20
@uw4 uw4 changed the title fix: extract mutable eval state into EvalContext — fixes #90, #93 fix: preserve the context state during evaluate — fixes #90, #93 Apr 29, 2026
@uw4 uw4 self-assigned this Apr 29, 2026
@uw4 uw4 merged commit 3168b6a into dashjoin:main Apr 29, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Evaluation of different Jsonata instances in the same thread causes issue incorrect eval with deep (ThreadLocal context)

2 participants